-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Further improval of controls #42
Further improval of controls #42
Conversation
These two methods were not well-thought. While a control that has the read-only flag set is certainly not writeable, a control with the write-only flag set could still be unwriteable because of being inactive. The original intention for these two methods was to check for the flags, so adjust their behaviour and naming accordingly.
Before 000696e, a control was assumed (and reported by this method) to be writeable when the read-only was not set. However, such a control would still be unwriteable when being flagged as inactive, which was not taken into account before. In other words, is_writeable now checks that a control can *actually* be written to.
This will be used later to facilitate instantiation of a Controls object with diverse Control sub-classes.
…oved into a method of class Control
This commit covers the part up to LegacyControl, as discussed in #15. The upgrade path for existing code is: Use LegacyControl instead of Control for instantiations, and use either BaseControl or BaseSingleControl for isinstance() checks (see Controls).
is_<flag> is working for many of the flags, but is weird for stuff like V4L2_CTRL_FLAG_HAS_PAYLOAD or V4L2_CTRL_FLAG_UPDATE. Switching to is_flagged_<flag> to check if a flag is set, e.g. is_flagged_has_payload, is still not perfect, but better.
…ntrol's __repr__, too
…l doesn't have the attributes minimum and maximum.
…hich is used by default.
…tiation of different control class objects without a messy if/elif construct.
…m optional (default: on)
…efore trying to mangle it
This class inherits from UserDict and thus can be used like a dict to access the items defined for the menu. For the sake of this dict-like behaviour the MenuItem class is not used in MenuControl; the names of the menu items will be transformed straight to either string or integer, depending on the menu type. MenuItem is kept for now, but renamed to LegacyMenuItem to signal it should not be used except for legacy code.
It turns out that my option 2 matches what the Pro Git book describes being "the GitHub workflow". I think I'll try that path and see where it ends :-) |
…egacyControl for control types not found in ctrl_type_map.
the rest to the new derived class BaseMonoControl. The latter is more or less a reprise of BaseSingleControl, which had been removed in 5640d4e. It's similar but not identical, and used to prepare for the upcoming ButtonControl.
no real device that provides a button control and trying vivid did not work (see #17).
Ok, with a9d5f3a I think I now have covered everything I originally wanted to have in this PR. To summarize, the idea is to provide a more pythonic API for working with a device's controls. See otaku42#12 for the initial motivation, and otaku42#15 for some kind of rationale. A basic comparison of the old vs. the new API is given in README.md, and with v4l2py-ctl.py there is an example of how to use the new API. Please let me know if you have any concerns, suggestions or questions. |
Thanks for these awesome improvements @otaku42! 🤩 I tried the examples/web with your branch but they fail with:
would it be possible to include a fix? Thanks in advance |
Oops. Thanks for pointing that out, I'll fix it. I didn't test the examples by now, to be honest, but was about to do that soon. I didn't expect they make use of the controls API. The info-attribute of controls has been renamed to _info, to signal that while it can be accessed it's on ones own risk. Sub-attributes of info are made available as top-level attributes if and where it makes sense. For example, minimum or maximum may or may not be available depending on the control type - see otaku42#12, which is what triggered this whole controls improvement thing :-) |
is named opencv-python; call pip as module, as suggested in the pip manual. Closes #22.
I've taken the lazy approach to fix the issue you have reported; see a98df12 . While working on it, I came across some issues:
and had some ideas for slight enhancements for I didn't want to cram these things into the PR, but I'd try to fix them at a later time, after this PR has been merged. |
Thanks for the fix and the extra work to keep example working with python 3.7. It is an honor to accept your PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Merging
It's my pleasure, and I'm glad that I can contribute a tiny bit to this great project. Thanks for the great work - and for accepting my contributions :-) |
Hi @tiagocoutinho . I'm still fairly new to Git and collaboration here on Github, so I thought I'd ask you for your input.
I've worked a bit to further improve controls (see this Draft PR), which happened in the improve_controls branch in otaku42/v4l2py, and I would like to contribute that work upstream.
While I did my stuff, you have committed a nice bunch of changes. This results in improve_controls being 35 commits ahead, 65 commits behind of your master branch. Merging is not easily possible due to merge conflicts that need to be resolved. So far, so good - but what is the best way to do this?
The "sync fork" tool on Github would discard my 35 commits "to make [the improve_controls] branch match the upstream repository". Although it just might be the explanation what happens during a merge anyway, I don't feel comfortable with trying what happens next, as I don't want to mess the repository/branch.
I'd rather resort to merging things locally and then pushing the results to Github, and I see the following options for that:
Assuming that you're willing to accept the changes, which of these options would you consider to be the best way to contribute them? Or are there other, better options that I've missed? I'd appreciate any suggestion.